-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Kedro in Azure ML with SQLiteSessionStore #2131
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
SajidAlamQB
requested review from
rashidakanchwala and
ravi-kumar-pilla
as code owners
October 9, 2024 12:12
Signed-off-by: Sajid Alam <[email protected]>
SajidAlamQB
changed the title
Fix Kedro in Azure with SQLiteSessionStore
Fix Kedro in Azure ML with SQLiteSessionStore
Oct 9, 2024
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
rashidakanchwala
approved these changes
Oct 11, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. thanks
jitu5
approved these changes
Oct 14, 2024
Signed-off-by: Sajid Alam <[email protected]>
Signed-off-by: Sajid Alam <[email protected]>
Huongg
approved these changes
Oct 14, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @SajidAlamQB , LGTM too 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Related to: #1991
This PR addresses issues with SQLite database locking in Azure Machine Learning (AML).
Reading online other users have also had this problem. It stems from the fact that in Azure ML the
/home
directory is mounted as CIFS filesystem which can't deal with SQLite3 lock. Read more here: https://stackoverflow.com/a/54051016To resolve this, we can enable Write-Ahead Logging (WAL) mode to fix these locking issues.
We can either:
Adopt WAL for all environments: If we go with this option the
session_store.db
file will have a small change in its internal structure to indicate that it is now using WAL mode. This change is stored in the database's header.Enable WAL mode conditionally for Azure ML: So we activate WAL mode only when the code detects that it is running in an AML environment. (Currently implemented in this PR).
Development notes
Included
PRAGMA journal_mode=WAL;
to enableWAL
mode.Implemented a condition to detect when it is running in an Azure Machine Learning environment using environment variables specific to Azure:
AZUREML_ARM_SUBSCRIPTION
,AZUREML_ARM_RESOURCEGROUP
, orAZUREML_RUN_ID
.Provided two options:
is_azure_ml
flag.QA notes
I tested this in Azure Machine Learning studio and confirmed that the database operates in WAL mode without the locking issues.
Tested locally as well it works as expected with and without enabling WAL mode.
Checklist
RELEASE.md
file